-
Notifications
You must be signed in to change notification settings - Fork 1.2k
add workspace-hack crate using cargo-hikari #1326
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughA workspace-hack crate is established using cargo hakari to optimize build times and improve dependency caching. New configuration and a hakari-generated crate are added, with all workspace crates updated to depend on it. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/rendering/Cargo.toml (1)
1-43: Fix invalid Rust edition.The workspace-hack setup is properly configured with the crate correctly defined at
crates/workspace-hack/Cargo.toml, Hakari configuration in place, and 39 workspace members declaring the dependency. However,crates/rendering/Cargo.tomlline 4 specifiesedition = "2024", which is invalid. Valid Rust editions are"2015"and"2021". Change toedition = "2021".crates/cpal-ffmpeg/Cargo.toml (1)
4-4: Invalid Rust edition: "2024" does not exist.Rust editions are 2015, 2018, or 2021. Edition "2024" is not valid and will cause build failures. This appears across all crates in this PR.
-edition = "2024" +edition = "2021"This must be corrected in all affected crates before merge.
♻️ Duplicate comments (7)
crates/gpu-converters/Cargo.toml (1)
4-4: Invalid Rust edition: "2024" does not exist.See related comment in crates/cpal-ffmpeg/Cargo.toml—this edition value must be corrected to "2021" across all affected crates.
crates/scap-cpal/Cargo.toml (1)
4-4: Invalid Rust edition: "2024" does not exist.See related comment in crates/cpal-ffmpeg/Cargo.toml—this edition value must be corrected to "2021" across all affected crates.
crates/ffmpeg-hw-device/Cargo.toml (1)
4-4: Invalid Rust edition: "2024" does not exist.See related comment in crates/cpal-ffmpeg/Cargo.toml—this edition value must be corrected to "2021" across all affected crates.
crates/editor/Cargo.toml (1)
4-4: Invalid Rust edition: "2024" does not exist.See related comment in crates/cpal-ffmpeg/Cargo.toml—this edition value must be corrected to "2021" across all affected crates.
crates/mediafoundation-ffmpeg/Cargo.toml (1)
4-4: Invalid Rust edition: "2024" does not exist.See related comment in crates/cpal-ffmpeg/Cargo.toml—this edition value must be corrected to "2021" across all affected crates.
crates/enc-avfoundation/Cargo.toml (1)
4-4: Invalid Rust edition: "2024" does not exist.See related comment in crates/cpal-ffmpeg/Cargo.toml—this edition value must be corrected to "2021" across all affected crates.
crates/scap-direct3d/Cargo.toml (1)
4-4: Invalid Rust edition: "2024" does not exist.See related comment in crates/cpal-ffmpeg/Cargo.toml—this edition value must be corrected to "2021" across all affected crates.
🧹 Nitpick comments (3)
Cargo.toml (1)
3-3: Consider removing redundant workspace member.The
crates/workspace-hackmember is redundant since the glob patterncrates/*already includes all crates in that directory. While Cargo will deduplicate this, explicitly listing it may cause confusion.Apply this diff if you'd like to simplify:
-members = ["apps/cli", "apps/desktop/src-tauri", "crates/*", "crates/workspace-hack"] +members = ["apps/cli", "apps/desktop/src-tauri", "crates/*"]crates/editor/Cargo.toml (1)
28-28: Consider alphabetical ordering of dependencies.The workspace-hack dependency is appended at the end, whereas other dependencies follow a looser grouping (workspace dependencies, then external crates). For consistency and maintainability, consider placing it alphabetically or grouping it explicitly with a comment.
crates/workspace-hack/Cargo.toml (1)
1-3: Do not manually edit the HAKARI SECTION.Lines 1-3 clearly indicate this file is generated by
cargo hakariand must be regenerated usingcargo hakari generate. The dependencies between lines 16-117 are automatically managed. Any manual edits will be lost when hakari regenerates the file. To update dependencies, modify workspace crate manifests and re-run hakari generation.Have you run
cargo hakari generateafter making all workspace crate changes? If not, this file may be out of sync with actual workspace dependencies.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (45)
.config/hakari.toml(1 hunks)Cargo.toml(1 hunks)apps/cli/Cargo.toml(1 hunks)apps/desktop/src-tauri/Cargo.toml(1 hunks)crates/api/Cargo.toml(1 hunks)crates/audio/Cargo.toml(1 hunks)crates/camera-avfoundation/Cargo.toml(1 hunks)crates/camera-directshow/Cargo.toml(1 hunks)crates/camera-ffmpeg/Cargo.toml(1 hunks)crates/camera-mediafoundation/Cargo.toml(1 hunks)crates/camera-windows/Cargo.toml(1 hunks)crates/camera/Cargo.toml(1 hunks)crates/cpal-ffmpeg/Cargo.toml(1 hunks)crates/cursor-capture/Cargo.toml(1 hunks)crates/cursor-info/Cargo.toml(1 hunks)crates/editor/Cargo.toml(1 hunks)crates/enc-avfoundation/Cargo.toml(1 hunks)crates/enc-ffmpeg/Cargo.toml(1 hunks)crates/enc-gif/Cargo.toml(1 hunks)crates/enc-mediafoundation/Cargo.toml(1 hunks)crates/export/Cargo.toml(1 hunks)crates/fail/Cargo.toml(1 hunks)crates/ffmpeg-hw-device/Cargo.toml(1 hunks)crates/flags/Cargo.toml(1 hunks)crates/gpu-converters/Cargo.toml(1 hunks)crates/media-info/Cargo.toml(1 hunks)crates/media/Cargo.toml(1 hunks)crates/mediafoundation-ffmpeg/Cargo.toml(1 hunks)crates/mediafoundation-utils/Cargo.toml(1 hunks)crates/project/Cargo.toml(1 hunks)crates/recording/Cargo.toml(1 hunks)crates/rendering-skia/Cargo.toml(1 hunks)crates/rendering/Cargo.toml(1 hunks)crates/scap-cpal/Cargo.toml(1 hunks)crates/scap-direct3d/Cargo.toml(1 hunks)crates/scap-ffmpeg/Cargo.toml(1 hunks)crates/scap-screencapturekit/Cargo.toml(1 hunks)crates/scap-targets/Cargo.toml(1 hunks)crates/timestamp/Cargo.toml(1 hunks)crates/utils/Cargo.toml(1 hunks)crates/video-decode/Cargo.toml(1 hunks)crates/workspace-hack/.gitattributes(1 hunks)crates/workspace-hack/Cargo.toml(1 hunks)crates/workspace-hack/build.rs(1 hunks)crates/workspace-hack/src/lib.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
crates/workspace-hack/src/lib.rscrates/workspace-hack/build.rs
crates/*/src/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Rust crates should place tests within the
src/and/or a siblingtests/directory for each crate insidecrates/*.
Files:
crates/workspace-hack/src/lib.rs
🧠 Learnings (2)
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to **/*.rs : Format Rust code using `rustfmt` and ensure all Rust code passes workspace-level clippy lints.
Applied to files:
crates/export/Cargo.tomlcrates/flags/Cargo.tomlCargo.tomlcrates/api/Cargo.tomlcrates/workspace-hack/Cargo.tomlcrates/workspace-hack/build.rs
📚 Learning: 2025-09-23T12:34:13.729Z
Learnt from: Brendonovich
Repo: CapSoftware/Cap PR: 1053
File: .github/workflows/publish.yml:222-230
Timestamp: 2025-09-23T12:34:13.729Z
Learning: In the Cap desktop app, there's a beforeBundle Tauri script (prodBeforeBundle.js) that creates Cap.dSYM in the target/ directory using dsymutil, so the Sentry debug symbols upload path "target/Cap.dSYM" is correct and doesn't need to be changed to the matrix target subdirectory.
Applied to files:
crates/rendering-skia/Cargo.toml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Clippy
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (42)
crates/workspace-hack/src/lib.rs (1)
1-1: LGTM! Stub library is correct for a workspace-hack crate.This minimal stub is the standard pattern for cargo-hakari workspace-hack crates, which exist solely to unify common dependencies across the workspace for better build caching.
crates/workspace-hack/.gitattributes (1)
1-4: LGTM! Correct git attributes for hakari-generated files.This configuration properly handles the hakari-generated Cargo.toml by treating it as binary during merges (avoiding conflict markers) and ensuring consistent line endings. This is the recommended setup for cargo-hakari.
crates/workspace-hack/build.rs (1)
1-2: LGTM! Minimal build script follows cargo-hakari convention.This empty build script is required for Cargo to process the
[build-dependencies]section in the workspace-hack crate's manifest. The implementation is correct.crates/enc-ffmpeg/Cargo.toml (1)
12-12: LGTM! Workspace-hack dependency correctly added.The dependency on the workspace-hack crate is properly configured with the correct version and relative path.
crates/recording/Cargo.toml (1)
52-52: LGTM! Workspace-hack dependency correctly added.The dependency on the workspace-hack crate is properly configured with the correct version and relative path.
crates/camera-windows/Cargo.toml (1)
10-10: LGTM! Workspace-hack dependency correctly added.The dependency on the workspace-hack crate is properly configured with the correct version and relative path.
crates/media-info/Cargo.toml (1)
10-10: LGTM! Workspace-hack dependency correctly added.The dependency on the workspace-hack crate is properly configured with the correct version and relative path.
crates/fail/Cargo.toml (1)
8-8: Workspace-hack dependency added correctly.The dependency syntax and placement are correct. However, verification is needed to confirm the workspace-hack crate is properly defined.
crates/project/Cargo.toml (1)
22-22: Workspace-hack dependency added correctly.crates/camera-mediafoundation/Cargo.toml (1)
10-10: Workspace-hack dependency added correctly.crates/enc-gif/Cargo.toml (1)
12-12: Workspace-hack dependency added correctly.crates/audio/Cargo.toml (1)
12-12: Workspace-hack dependency added correctly.crates/camera-avfoundation/Cargo.toml (1)
19-19: Workspace-hack dependency added correctly.crates/cursor-capture/Cargo.toml (1)
13-13: Workspace-hack dependency added correctly.crates/cursor-info/Cargo.toml (2)
11-11: Workspace-hack dependency added correctly.
1-11: All verification checks passed—no issues found with the workspace-hack configuration or Rust 2024 edition.The workspace-hack crate is properly implemented with all required files, the root
Cargo.tomlcorrectly lists workspace-hack in the members, and.config/hakari.tomlis properly configured. Rust 1.85 was released with edition 2024, makingedition = "2024"a valid and current choice. The workspace-hack dependency is consistently added across the codebase as expected.apps/desktop/src-tauri/Cargo.toml (1)
116-116: Path depth verified for monorepo structure.The relative path correctly navigates from
apps/desktop/src-tauri/(3 levels deep) tocrates/workspace-hack/. This is consistent with the monorepo layout.crates/utils/Cargo.toml (1)
30-30: Path correctly resolves to sibling crate.The relative path is appropriate for a crate within the same
crates/directory.crates/scap-ffmpeg/Cargo.toml (1)
11-11: Path and version consistent across workspace.crates/timestamp/Cargo.toml (1)
10-10: Path and version consistent with other workspace crates.crates/video-decode/Cargo.toml (1)
14-14: Consistent with workspace pattern.crates/flags/Cargo.toml (1)
9-9: Dependency follows established pattern.crates/media/Cargo.toml (1)
14-14: Consistent with workspace integration.crates/rendering/Cargo.toml (1)
35-35: Consistent with workspace pattern.crates/cpal-ffmpeg/Cargo.toml (1)
9-9: Workspace-hack dependency path is correct.The relative path
../workspace-hackresolves correctly fromcrates/cpal-ffmpeg/tocrates/workspace-hack/.crates/scap-direct3d/Cargo.toml (1)
42-44: Verify Cargo.toml section ordering.The
[dependencies]section appears after[target.'cfg(windows)'.dependencies]and[dev-dependencies]. Standard Cargo.toml convention places[dependencies]near the beginning. This may indicate an unintended ordering issue during the file edit. Verify the intended structure.crates/enc-mediafoundation/Cargo.toml (1)
10-10: ✓ Correct Hakari dependency format.The workspace-hack dependency declaration follows the expected format version "2" pattern:
workspace-hack = { version = "0.1", path = ... }. Relative path is accurate for this crate's location.crates/mediafoundation-utils/Cargo.toml (1)
7-7: ✓ Correct Hakari dependency format.The workspace-hack dependency declaration follows the expected pattern. Relative path is accurate for this crate's location.
apps/cli/Cargo.toml (1)
26-26: ✓ Correct path for apps/ directory location.The workspace-hack dependency correctly uses
../../crates/workspace-hackto account for this crate's location underapps/, while other crates undercrates/use../workspace-hack.crates/rendering-skia/Cargo.toml (1)
21-21: ✓ Correct Hakari dependency format.The workspace-hack dependency is correctly placed and the relative path is accurate for this crate's location.
crates/export/Cargo.toml (1)
31-31: ✓ Correct Hakari dependency format.The workspace-hack dependency is correctly placed and the relative path is accurate for this crate's location.
crates/api/Cargo.toml (1)
9-9: ✓ Correct Hakari dependency format.The workspace-hack dependency is correctly placed and the relative path is accurate for this crate's location.
crates/scap-targets/Cargo.toml (1)
14-14: ✓ Correct Hakari dependency format.The workspace-hack dependency is correctly placed and the relative path is accurate for this crate's location.
crates/camera/Cargo.toml (1)
11-11: ✓ Correct Hakari dependency format.The workspace-hack dependency is correctly placed and the relative path is accurate for this crate's location.
.config/hakari.toml (2)
19-24: Verify platform selection is intentional.The platforms array includes
aarch64-apple-darwinandx86_64-pc-windows-msvc, butx86_64-unknown-linux-gnuis commented out. Confirm this platform selection aligns with your CI/development environment needs. If Linux builds are part of your CI, consider enabling it.
1-27: Now let me check the repository for CI configuration and build setup:Based on my verification, the original review comment's concern is incomplete. Starting cargo hakari 0.9.22, .config/hakari.toml files created by cargo hakari init set it to "4". The configuration file already contains a comment documenting this requirement on line 7.
After changing the config, you should run cargo hakari generate, and if either cargo hakari generate --diff or cargo hakari manage-deps --dry-run exits with a non-zero status, you can choose to fail CI or produce a warning message. This means the version requirement is enforced when CI runs hakari commands, not through explicit version specification in dependency files.
The review comment's suggestion to "ensure this version is available" lacks specificity—cargo-hakari is a build tool typically installed at CI time, not a workspace dependency. The configuration file is self-documenting with its existing comment.
No actionable changes are needed for .config/hakari.toml. If a concern exists about hakari version enforcement, it should focus on CI setup, not the configuration file itself.
crates/camera-ffmpeg/Cargo.toml (1)
11-11: Consistent workspace-hack dependency pattern.The dependency specification matches the pattern used in other crates. Ensure this crate's path resolution works correctly (
../workspace-hackfromcrates/camera-ffmpeg/).crates/camera-directshow/Cargo.toml (1)
10-10: Consistent workspace-hack dependency pattern.The dependency specification is consistent with other workspace crates. Path resolution should work correctly.
crates/workspace-hack/Cargo.toml (3)
5-11: Package configuration is correct for a workspace hack crate.The manifest correctly declares this as a non-publishable workspace utility with appropriate metadata. Edition 2021 is a stable choice.
1-117: Files verified: build.rs and src/lib.rs are present and committed.Both required files exist in the workspace-hack crate:
crates/workspace-hack/build.rscontains the required build script stubcrates/workspace-hack/src/lib.rsis present and properly placedThe verification confirms the files are committed and have appropriate minimal content for a hakari-managed workspace-hack package.
39-39: No issues found. The version fragmentation is correctly handled by hakari.The workspace-hack Cargo.toml properly manages unavoidable package version conflicts. The itertools fragmentation (0.12 vs 0.13) stems from incompatible bindgen versions: bindgen 0.69.5 requires itertools 0.12, while bindgen 0.70.1+ requires itertools 0.13. Different workspace dependencies (libspa-sys, pipewire-sys, whisper-rs-sys vs. coreaudio-sys, ffmpeg-sys-next, skia-bindings) pull different bindgen versions. Similarly, phf_shared versions are driven by transitive dependency conflicts. The renamed package identifiers enable these incompatible versions to coexist, which is the intended behavior. No consolidation is possible without breaking some dependency chains.
crates/scap-screencapturekit/Cargo.toml (1)
10-10: workspace-hack dependency is properly configured.The workspace root
Cargo.tomlcorrectly includescrates/workspace-hackin the members list, the directory exists, and the version reference matches. The dependency path and version are both correct.
Summary by CodeRabbit